[BE] 레이싱게임_고준보 미션 연습 #520

Ridealist · 2022-12-03T07:12:42Z

코드 리뷰 부탁드립니다!
저도 맞리뷰 가겠습니다:)

Input에서 사용자 이름을 받아 Player 객체 생성하도록 한 라운드 진행할 때 playOneRound 메세지 받아 수행 ParameterizedTest 사용해 단위 테스트 구현

게임 라운드 계속 진행 여부 관리 메서드, 게임 진행 메서드 구현 게임 진행에 따른 메서드 반환값 변환 단위 테스트 구현

콤마 포함 여부, 이름 길이 검사, 숫자 여부 검사 메소드 구현

view에서 입력값 받아 옴, 잘못된 입력값에 error 메세지 출력 후 재요청 domain 객체들 생성 및 연결 view에서 게임 상황 및 결과 출력 메소드 호출

khj1

안녕하세요 준보님! 맞리뷰 왔습니다 :)

스트림을 활용하는 방식과 객체 분리에 대해서만 더 공부해보시면 훨씬 좋은 코드를 작성하실 수 있을 것 같아요!

저도 객체 분리에 대해서 고민을 정말 많이했는데요! <객체지향의 사실과 오해> 라는 책이 정말 많은 도움이 됐어요! 책을 직접 읽으시는 게 좋지만 그럴 여건이 안되신다면

생각하라, 객체지향 처럼

이 자료를 읽어보시는 것을 강력히 추천드립니다!

또한 스트림에 대해서도 공부해보시면 좋을 것 같아요! 개인적으로 <모던 자바 인 액션>을 직접 읽어보시는 것을 추천드려요! 저도 지금 읽고 있지만 정말 많은 인사이트를 얻고 있습니다.

제가 정리한 내용 중에 준보님에게 지금 당장 도움이 될만한 내용을 첨부 해드리겠습니다!

모던 자바 인 액션 ch5 - 스트림 활용

다음 자료들도 함께 보시면 더 좋을 것 같습니다!!!

너무 주절 주절 말이 길어졌네요!
리뷰 해주셔서 감사합니다 준보님 항상 응원할게요! :)

khj1 · 2022-12-03T11:57:39Z

src/main/java/racingcar/domain/GameResult.java

+public class GameResult {
+
+    private final Player player;
+    private Map<String, Integer> playerPosition = new LinkedHashMap<>();


일급 컬렉션을 사용하는 것도 좋다고 생각해요!

일급 컬렉션을 사용하는 이유

추가로 playerPosition 상태를 활용하신 이유가 궁금합니다!

게임이 진행될 때마다 Car 객체 내부에서 name 과 position 값을 불러와서 저장하는 용도로 사용하고 계신 것 같아요. 마치 repository 처럼요! 그런데 굳이 실시간으로 변하는 position 값을 따로 저장할 필요가 있을 지 고민해보시는 게 좋을 것 같아요! 이미 Car 객체가 최신화 된 position 값을 저장하고 있기 때문에 이를 활용하시는 게 더 합리적일 것 같습니다!

게임 결과를 편하게 출력하기 위한 용도로 playerPosition 을 사용하신 건 아닐까? 라는 생각도 드네요. 그럼에도 GameResult 보다는 RacingGame 이나, Player 객체에서 라운드 마다 게임 결과를 반환하는 책임을 수행하는 게 더 적절해 보여요!

GameResult 와 RacingGame 두 객체 모두 Player에 의존하고 있어요. 이런 경우에 해당 객체에 적절한 책임이 부여됐는지 한번 고민해보시면 좋을 것 같습니다!

khj1 · 2022-12-03T12:00:22Z

src/main/java/racingcar/domain/car/Car.java

+    }
+
+    public void moveOrStop(NumberGenerator numberGenerator) {
+        if (numberGenerator.generate() >= MOVING_CRITERIA) {


해당 조건문을 메서드로 분리해서 어떤 역할을 하는 지 이름을 지어주는 것이 좋다고 생각해요!

한준님 말씀에 동의합니다. moveOrStop 이름이 1.이동할 수 있을지 판단하는 역할과 2.자동차를 이동시키는 역할 2가지를 함께 하고 있어, 한가지 역할만 해야하는 메소드 원칙에 위배되었네요... 피드백 감사합니다:)

khj1 · 2022-12-03T12:11:41Z

src/main/java/racingcar/domain/Player.java

+import racingcar.domain.car.Car;
+import racingcar.domain.car.NumberGenerator;
+
+public class Player {


Player 내부에서 복수의 Car 객체들을 다루고 있네요!
Player 보단 Players 처럼 복수형으로 이름을 지어주시는 게 더 타당해보입니다!

네 동의합니다. 일반적으로 클래스명에 단수를 써야한다는(?) 저만의 강박이 있었는데, 한준님 피드백 덕분에 제 고정관념을 깨게 되었습니다. stackoverflow에서도 제 생각과 비슷한 고민을 한 분이 있었네요...ㅎㅎ 감사합니다!
https://stackoverflow.com/questions/11673750/is-it-good-practice-for-java-class-names-to-be-plural

khj1 · 2022-12-03T12:15:56Z

src/main/java/racingcar/domain/Player.java

+    public List<Car> getRacingCars() {
+        return racingCars;
+    }


getter()로 리스트를 그대로 반환하는 것인 정말 위험하다고 생각해요!
리스트를 반환하더라도 실제로 반환되는 것은 해당 리스트의 참조 값입니다.

만약 다음과 같은 코드가 작성되면 외부에서 Player 내부의 상태가 쉽게 바뀔 수 있어요.

List<Car> cars = player.getRacingCars(); cars = new ArrayList<Car>();

따라서 리스트를 반환할 때는 불변 객체로 변환해주시는 게 안전해요!
Collections.unmodifiableList() 를 활용해보시는 것을 추천드립니다!

한준님의 코드에서 이 부분의 의도가 궁금했는데, 이런 깊은 뜻이 있었군요! 조언 정말 감사합니다:)

한준님 덕분에 더 공부해봤는데, List 필드에 값을 지정할 때 아예 unmodifiableList를 적용해주면 어떨까 싶어
자료들 찾아봤고, 아래 자료 읽어보면 좋을 것 같아 공유드립니다:)
남겨주신 리뷰로 많은 공부 했습니다 감사합니다!

UnmodifiableList은 만능이 아니다. / 방어적 복사

좋은 자료 감사합니다!! 일급 컬렉션을 반환할 때 뿐만 아니라 생성할 떄도 불변성을 고려해줘야 하는군요...!
덕분에 방어적 복사에 대해서도 알 수 있었어요! 정말 감사합니다!!!! 👍👍 👍 👍 👍

khj1 · 2022-12-03T12:38:12Z

src/main/java/racingcar/controller/GameController.java

+        InputView inputView = new InputView();
+        List<String> playerNames = repeat(inputView::readPlayerName);
+        Player player = new Player(playerNames);
+        this.gameResult = new GameResult(player);
+        int gameRound = repeat(inputView::readGameRound);
+        this.racingGame = new RacingGame(player, gameRound);


setUp() 메서드 내부의 기능들도 분리해보시는 것이 어떨까요?

setUp() 메서드는 결국 RacingGame 생성을 최종 목표로 하고 있네요.
그렇다면 RacingGame 이 생성되는 과정들을 적절하게 묶어서 메서드로 분리해보시는 것이
가독성 측면에서 더 좋을 것 같습니다!

khj1 · 2022-12-03T12:42:31Z

src/main/java/racingcar/utils/InputValidator.java

+    private static final String NOT_IN_COMMA = "자동차 이름을 쉼표(,)로 구분해 적어주세요.";
+    private static final String INVALID_NAME_LENGTH = "자동차 이름을 5글자 이내로 입력해주세요.";
+    private static final String NOT_NUMBER = "시도할 횟수를 숫자로 입력해주세요.";


InputValidator 에서 거의 모든 예외를 전부 처리하고 있어요.

자동차 이름 길이와 같은 제약 사항은 도메인에서 관리해주는 것이 더 타당하다고 생각합니다!
입력 단계에서 관리해 줄 예외와 도메인 단에서 관리해 줄 예외를 구분해보시는 것을 추천드려요!

한준님 말씀에 동의합니다.

저도 모든 validation을 InputValidator에서 처리하는 구조가 BE 입장에서도 안 좋다고 생각하긴 했습니다:)
현준님 말씀처럼 비즈니스 로직 문제도 그렇고, view는 결국 FE 단인데 FE에서 넘어온 데이터에 대한 유효성 검증 책임이 BE에도 있다고 생각해서요ㅎㅎ 다만 그 Exception 처리를 controller에서 더 쉽게 하기 위해, 어쩔 수 없이 InputValidator에 모든 책임을 맡겼습니다.

현준님 코드 보면서 controller에서 도메인 객체를 생성하는 부분만 메소드화 시켜 exception 처리 메소드에 대입해서 사용하신걸 보고 적절한 처리 방법을 배웠습니다. 감사합니다!

khj1 · 2022-12-03T12:46:49Z

src/main/java/racingcar/view/InputView.java

+        InputValidator.notContainsComma(input);
+        String[] inputList = InputValidator.moreThanLenFive(input);


메서드 네임에 Comma 와 같은 너무 구체적인 사항이 들어가는 것은 좋지 않다고 생각해요.
만약에 자동차 이름을 구분하는 단위가 , 에서 : 로 변했다면 관련된 메서드 이름까지 바꿔줘야 하는 번거로움이 있습니다.

moreThanLenFive 도 마찬가지로 자동차 이름을 6자까지 허용해준다면? 메서드 네이밍이 변해야겠죠.
이처럼 이름에 자료형이나 구체적인 값을 포함시키는 것은 유지보수에 매우 불리하게 작용합니다!

추가로 이름을 축약시키는 것도 지양해야한다고 알고있어요! 이름이 길어지더라도 Len 보다는 Length 를 그대로 사용하시는 것이 더 이해하기 쉽다고 생각합니다!

맞습니다. 이건 부끄럽네요ㅎㅎ...
예전에 다른 분께도 메소드 네이밍에 대해 지적받은 부분인데 아직 고치지 못하고 있었습니다.
말씀하신 내용 다 동의하고, 지나치게 구체적인 메서드 네이밍이나 축약 표현 지향하도록 노력하겠습니다.
감사합니다!

khj1 · 2022-12-03T12:49:08Z

src/test/java/racingcar/domain/CarTest.java

+        NumberGenerator numberGenerator = new TestNumberGenerator(3);
+        Car car = new Car("June");


테스트 코드에서 반복적으로 사용되는 값이나 객체를 어떻게 처리할 지
고민해보시는 것도 좋을 것 같아요!

@BeforeEach 와 같은 테스트 라이프 사이클을 활용해보시는 건 어떨까요?!

khj1 · 2022-12-03T12:54:20Z

src/test/java/racingcar/domain/PlayerTest.java

+        player.playOneRound(numberGenerator);
+        List<Car> racingCars = player.getRacingCars();
+        for (Car car : racingCars) {
+            assertThat(car.getPosition()).isEqualTo(position);


getter() 지양하는 것이 좋지만 무조건 배제하는 것은 힘들다고 생각해요.
값을 출력해야할 때나 해당 값으로 연산을 진행해야할 때 어쩔 수 없이 getter()를 사용하게 되는 경우들이 있습니다.

다만 값을 비교할 때는 getter()를 활용하지 않고 메시지를 보내는 것 만으로 충분하다고 생각해요.
나랑 위치가 같니?, 나보다 위치가 더 머니? 와 같은 메시지들을 떠올려서 이에 맞는 메서드를 구현해보시는 것을 추천드려요!!

khj1 · 2022-12-03T12:56:06Z

src/test/java/racingcar/domain/PlayerTest.java

+    class TestNumberGenerator implements NumberGenerator {
+
+        private final int number;
+
+        public TestNumberGenerator(int number) {
+            this.number = number;
+        }
+
+        @Override
+        public int generate() {
+            return number;
+        }
+    }


TestNumberGenerator도 여러 코드에서 반복적으로 활용되고 있습니다.
Test 패키지 내에서 별도의 클래스로 분리하거나 따로 import 해서 중복을 제거해보시는 것은 어떨까요?

Ridealist added 21 commits November 30, 2022 18:47

docs: 구현할 기능 목록 정리

21c5320

feat: 전체 클래스 구조 밑그림 및 패키지 분리

b641c7a

feat: 전체 프로그램 skeleton code 구현

bb6e722

feat(NumberGenerator): 랜덤 숫자 생성 인터페이스 및 구현 클래스 구현

493ccd0

feat(Car): 랜덤 숫자에 따른 이동, 정지 메소드 구현

c4ca5c0

feat(Car): getter 메소드 구현

cf186f5

test(CarTest): 랜덤 숫자에 따른 이동 메서드 단위 테스트 구현

5a4a078

refactor: domain 내에 car 패키지 추가 생성, 클래스 분리

7aace4b

feat(Player): Car 객체를 담아 전체 참가자를 관리하는 클래스 구현

c1407e9

Input에서 사용자 이름을 받아 Player 객체 생성하도록 한 라운드 진행할 때 playOneRound 메세지 받아 수행 ParameterizedTest 사용해 단위 테스트 구현

feat(RacingGame): RacingGame 관리하는 객체 구현

f2c0781

게임 라운드 계속 진행 여부 관리 메서드, 게임 진행 메서드 구현 게임 진행에 따른 메서드 반환값 변환 단위 테스트 구현

test(CarTest): 패키지 변화 적용 및 TODO 작성

688647f

feat(InputValidator): 입력값 유효성 검증 클래스 구현

ff0fafa

콤마 포함 여부, 이름 길이 검사, 숫자 여부 검사 메소드 구현

feat(InputView): 사용자 입력값 요청 클래스 구현

0c00aad

feat(OutputView): 콘솔 출력 로직 클래스 구현

91b6063

feat(GameResult): 게임 상황 및 최종 결과 반환 클래스 구현

3ef0908

fix(InputView): player 이름에 빈칸 출력되는 문제 해결

2a30bbc

chore: exception 메세지 내용 수정

aee2952

chore: exception 메세지 내용 수정, 빈칸 추가

a4af3a4

feat(GameController): controller 클래스 구현

3bbd941

view에서 입력값 받아 옴, 잘못된 입력값에 error 메세지 출력 후 재요청 domain 객체들 생성 및 연결 view에서 게임 상황 및 결과 출력 메소드 호출

feat(Application): 전체 기능 구현 완료

95808b3

docs: 구현할 기능 목록 전체 체크 완료

68e5ee5

khj1 reviewed Dec 3, 2022

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[BE] 레이싱게임_고준보 미션 연습 #520

[BE] 레이싱게임_고준보 미션 연습 #520

Ridealist commented Dec 3, 2022

khj1 left a comment

khj1 Dec 3, 2022

khj1 Dec 3, 2022

Ridealist Dec 4, 2022

khj1 Dec 3, 2022

Ridealist Dec 4, 2022 •

edited

Loading

khj1 Dec 3, 2022

Ridealist Dec 4, 2022 •

edited

Loading

Ridealist Dec 4, 2022

khj1 Dec 4, 2022

khj1 Dec 3, 2022

khj1 Dec 3, 2022

Ridealist Dec 4, 2022

khj1 Dec 3, 2022

Ridealist Dec 4, 2022

khj1 Dec 3, 2022

khj1 Dec 3, 2022

khj1 Dec 3, 2022

		InputValidator.notContainsComma(input);
		String[] inputList = InputValidator.moreThanLenFive(input);

		NumberGenerator numberGenerator = new TestNumberGenerator(3);
		Car car = new Car("June");

[BE] 레이싱게임_고준보 미션 연습 #520

Are you sure you want to change the base?

[BE] 레이싱게임_고준보 미션 연습 #520

Conversation

Ridealist commented Dec 3, 2022

khj1 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Ridealist Dec 4, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Ridealist Dec 4, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Ridealist Dec 4, 2022 •

edited

Loading

Ridealist Dec 4, 2022 •

edited

Loading